chore: E2E test for checking if Plonk parameters require regenerating #69
Conversation
storojs72
force-pushed
the
check-parameters
branch
from
2b33430
to
eb542bc
Compare
storojs72
changed the title
storojs72
force-pushed
the
check-parameters
branch
from
eb542bc
to
c2f760f
Compare
There was a problem hiding this comment.
The new test seems good to me in principle and it makes sense, but just to double check: did you mark it as #[ignore] so it wouldn't take too long to run in CI? That is, the test is meant to be run manually and locally?
test_e2e takes ~800s to run in CI currently, so adding test_e2e_check_parameters would probably double that, I believe, so marking it as #[ignore]d is a good call.
One thing to keep in mind about the test as it's written is that, if the parameters already exist (e.g. you generated them locally previously to running the test), the test won't try to re-download them.
|
With this PR and our S3 buckets being public, issue #35 now is only about making a release workflow for the S3 artifacts, and argumentcomputer/zk-light-clients#35 should no longer be a problem. |
|
Yes, when sphinx is updated and Plonk circuit is changed, its parameters no longer work and this test identifies it by failing with constraints error. Then running this test again after uploading newer Plonk parameters to AWS and getting success allows to check that new parameters actually work. |
…#69) * chore: Add e2e test for checking newly installed Plonk parameters * chore: Use public address of AWS bucket
…#69) * chore: Add e2e test for checking newly installed Plonk parameters * chore: Use public address of AWS bucket
I found it useful running this newly introduced test (
test_e2e_check_parameters) to check if installed Plonk parameters work with current version of sphinx (when particular PR that potentially modifies the Plonk circuit has been merged).As a side note, this PR includes reverting https access to AWS bucket, as now it is publicly accessed for reading